Skip to content

Add multi-user licenses (again)#802

Merged
ezekg merged 197 commits intomasterfrom
feature/add-multi-user-licenses-2
Apr 25, 2024
Merged

Add multi-user licenses (again)#802
ezekg merged 197 commits intomasterfrom
feature/add-multi-user-licenses-2

Conversation

@ezekg
Copy link
Member

@ezekg ezekg commented Mar 1, 2024 •
edited
Loading

Closes #534. Reverted #774 due to performance issues with union_of.

The issues are mainly i.r.t. INNER JOIN ... ON id IN (SELECT id FROM ...) joins. For large accounts, performance is just not where I want it. E.g. an account with 600k licenses takes 2s to load a user's machines, because we have to join across 600k licenses to see if the user has any associated licenses with other users, so that we can list the user's teammates' machines as well as the user's machines. The same issue is present when listing a user's licenses, and a user's teammates.

I tried optimizing a few queries by hand, but there's just too many joins on the users union association. We're going to need to optimize union_of itself so that we don't do an JOIN ... ON id IN (SELECT id FROM ...), since that could potentially select hundreds of thousands or even millions of IDs, which is not memory efficient (not to mention slow).

This is what I get for skipping performance testing.

Prerequisites

Pre-deploy

  • Create license.users.attached, license.users.detached, license.owner.updated, machine.owner.updated event types.
  • Create license.users.attach, license.users.detach, license.owner.update, machine.owner.update permissions.
  • Optimize autovacuum settings (especially the users table).
  • Manually vacuum+analyze users and licenses tables.
  • Enable query logging for denormalization migration.

Post-deploy

  • Run VACUUM ANALYZE licenses.
  • Run VACUUM ANALYZE users.
  • Run the below Rake tasks.
# Add new permissions and event types
rake db:seed

# Migrate existing role permissions
rake keygen:permissions:admins:add[license.users.attach,license.users.detach,license.owner.update,machine.owner.update]
rake keygen:permissions:environments:add[license.users.attach,license.users.detach,license.owner.update,machine.owner.update]
rake keygen:permissions:products:add[license.users.attach,license.users.detach,license.owner.update,machine.owner.update]

@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 3 times, most recently from 0c02fda to bdbf11d Compare March 9, 2024 08:40
ezekg
ezekg commented Mar 11, 2024
View reviewed changes
ezekg
ezekg commented Mar 12, 2024
View reviewed changes
@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 3 times, most recently from b5bedcc to 9c440e4 Compare March 20, 2024 01:25
@ezekg
Copy link
Member Author

ezekg commented Mar 23, 2024

Here are some queries that seem to perform well, and are able to be indexed efficiently:

-- licenses for user
select licenses.* from licenses left outer join license_users on license_users.license_id = licenses.id and license_users.user_id = '598840ca-c529-40fd-9d9b-fe650619726a' and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and (licenses.user_id = '598840ca-c529-40fd-9d9b-fe650619726a' or license_users.user_id = '598840ca-c529-40fd-9d9b-fe650619726a') order by licenses.created_at desc limit 10;

-- users for product
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join policies on policies.id = licenses.policy_id and policies.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and policies.product_id = '00dda7a7-deb7-4a76-8a84-2be19adde374' order by users.created_at desc limit 10;

-- users for product (denormalized)
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and licenses.product_id = '00dda7a7-deb7-4a76-8a84-2be19adde374' order by users.created_at desc limit 10;

-- users for license
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and licenses.id = 'aa8ac994-a3ed-4c06-89fd-4c11736b6c55' order by users.created_at desc limit 10;

-- users for machine
select distinct users.* from users left outer join license_users on license_users.user_id = users.id and license_users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join licenses on (licenses.user_id = users.id or licenses.id = license_users.license_id) and licenses.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' inner join machines on machines.license_id = licenses.id and machines.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' where users.account_id = 'f98c3ce2-9373-498a-813a-d74b1506f6e7' and machines.id = 'db10ee10-a1ac-41c4-a7c9-332a534fe398' order by users.created_at desc limit 10;

@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 9 times, most recently from cf8ebce to f024ece Compare March 26, 2024 21:41
ezekg
ezekg commented Mar 26, 2024
View reviewed changes
spec/models/concerns/denormalizable_spec.rb
end
}

describe '.denormalizes' do
Copy link
Member Author

@ezekg ezekg Mar 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add more tests. Would be really nice to introduce temp tables for this and union_of.

Copy link
Member Author

@ezekg ezekg Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! https://github.com/keygen-sh/temporary_tables 😉

ezekg
ezekg commented Mar 27, 2024
View reviewed changes
@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch 3 times, most recently from 271eaa0 to af3e1a2 Compare March 27, 2024 21:22
@ezekg ezekg marked this pull request as ready for review March 27, 2024 21:24
@ezekg ezekg marked this pull request as draft March 27, 2024 21:25
ezekg
ezekg commented Mar 28, 2024
View reviewed changes
ezekg
ezekg commented Mar 28, 2024
View reviewed changes
ezekg
ezekg commented Mar 28, 2024
View reviewed changes
@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch from 6f148f6 to db611b3 Compare April 2, 2024 17:30
ezekg
ezekg commented Apr 2, 2024
View reviewed changes
ezekg added 28 commits April 24, 2024 20:54
@ezekg
fix test style
f5d478f
@ezekg
fix query matcher order of operations
232db67
@ezekg
add denormalization to
b416ddd
@ezekg
fix environmental/accountable asserts on denormalized associations
49b996d
@ezekg
fix order of denormalized product association
556e648
@ezekg
fix issue where some joins would use a leading join node
43f69e6
@ezekg
add default scopes to through unions
ff740ac
@ezekg
add test for query with default scopes
7d1dbbb
@ezekg
fix scopes utilizing potentionally nil denormalized foreign key
a71f284
@ezekg
remove backtrace cleaner hack
e461dd0
@ezekg
fix sql matcher formatting issue
6ed40dd
@ezekg
remove license denormalization hack
da80521
@ezekg
remove unused indexes
6c90dfa
@ezekg
update comments
9d13010
@ezekg
remove unused consts
e5aedbc
@ezekg
fix inactive user scope
3417a79
@ezekg
restructure scopes to filter earlier by inverting from/join
d0feedb
@ezekg
update where clauses for readability
62206a0
@ezekg
update users for product scope to use exists
d837347
@ezekg
add if not exists/if exists to multi-user migrations
f6fdf66 
- some of these have already been run in production
@ezekg
add covering index for user for product scope
a2ceefb
@ezekg
update new machine owner index to be concurrent
dc68416
@ezekg
add distinct to for user through scopes
628eef6
@ezekg
add distinct to release-related for user scopes
18b44b1
@ezekg
fix sort order of union scopes
1533b20
@ezekg
fix distinct order
1cedb03 
- unions are already distinct so we can push the distinct clause up
@ezekg
fix performance for user teammate ids reader
9b3e70c
@ezekg
refactor user active/inactive scopes for performance
40da878
@ezekg ezekg force-pushed the feature/add-multi-user-licenses-2 branch from 849c33f to 40da878 Compare April 25, 2024 01:55
@ezekg ezekg merged commit 3e02d87 into master Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

A license should have multiple users

1 participant

@ezekg